Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Helm chart updates related to Prometheus, Webhook HPA, and Flyteconsole probes #5508

Merged

Conversation

mhotan
Copy link
Contributor

@mhotan mhotan commented Jun 24, 2024

Why are the changes needed?

Various Helm chart updates deemed appropriate to upstream from Union internal usage.

  • Add flyteconsole livenessProbe and readinesProbe to mitigate downtime during releases or updates. Default to no probes to preserve backward-compatible behavior.
  • Add flyte-console prometheus port and service monitor
  • Add flyte-pod-webhook HPA
  • Add Prometheus annotations to propeller, webhook, and syncresources

How was this patch tested?

All three patches (commits) were tested individually within Union internal flyte deployment

Copy link

welcome bot commented Jun 24, 2024

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

  • Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
  • Sign off your commits (Reference: DCO Guide).

mhotan added 3 commits June 24, 2024 14:26
Some flyte components were lacking metrics. clustersyncresources is especially necessary now that new projects are created frequently in serverless. But we should be monitoring all internal services to some extent.

* Update clusterresourcesync to export metrics via httpendpoint
* Update clusterresourcesync deployment with Prometheus scrape annotations, disabled by default
* Update propeller deployment  with Prometheus scrape annotations, disabled by default
* Update webhook deployment  with Prometheus scrape annotations, disabled by default
* Create Webhook HPA, default disabled.

Changes are intended to be forward-compatible, specifically with Prometheus Pod and ServiceMonitor.

towards PE-912
towards PE-846

Signed-off-by: Mike Hotan <[email protected]>
* flyteconsole service monitor

Signed-off-by: Mike Hotan <[email protected]>
Make flyteconsole readiness & liveness probe configurable

Signed-off-by: Mike Hotan <[email protected]>
@mhotan mhotan force-pushed the mhotan/additional-prometheus-annotations branch from 713b7f5 to e5d89bd Compare June 24, 2024 21:26
@mhotan mhotan changed the title mhotan/additional prometheus annotations Helm chart updates related to Prometheus, Webhook HPA, and Flyteconsole probes Jun 24, 2024
Copy link

codecov bot commented Jun 25, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 61.01%. Comparing base (242303b) to head (e5d89bd).
Report is 123 commits behind head on master.

Files with missing lines Patch % Lines
flyteadmin/pkg/clusterresource/controller.go 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5508      +/-   ##
==========================================
- Coverage   61.01%   61.01%   -0.01%     
==========================================
  Files         794      794              
  Lines       51443    51445       +2     
==========================================
- Hits        31390    31388       -2     
- Misses      17161    17165       +4     
  Partials     2892     2892              
Flag Coverage Δ
unittests-datacatalog 69.31% <ø> (ø)
unittests-flyteadmin 58.73% <66.66%> (-0.01%) ⬇️
unittests-flytecopilot 17.79% <ø> (ø)
unittests-flytectl 68.03% <ø> (-0.06%) ⬇️
unittests-flyteidl 79.04% <ø> (ø)
unittests-flyteplugins 61.85% <ø> (ø)
unittests-flytepropeller 57.30% <ø> (ø)
unittests-flytestdlib 65.82% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@pingsutw pingsutw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@pingsutw pingsutw merged commit 3ee7120 into flyteorg:master Jun 25, 2024
55 checks passed
Copy link

welcome bot commented Jun 25, 2024

Congrats on merging your first pull request! 🎉

robert-ulbrich-mercedes-benz pushed a commit to robert-ulbrich-mercedes-benz/flyte that referenced this pull request Jul 2, 2024
vlibov pushed a commit to vlibov/flyte that referenced this pull request Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants